Add rules for operands in arithmetic operations #15
Conversation
|
If this one gets merged, I'd like to also add support for:
|
|
|
||
| public static function isValidForArithmeticOperation(Type $leftType, Type $rightType): bool | ||
| { | ||
| if (!$leftType instanceof IntegerType && !$leftType instanceof FloatType) { |
There was a problem hiding this comment.
This needs to use isSubtypeOf instead of instanceof.
There was a problem hiding this comment.
I guess I am a bit lost in this undocumented typesystem. :)
isSubtypeOf() is defined on CompoundType while Rule only accepts Type.
I tried:
$acceptedType = new UnionType([new IntegerType(), new FloatType()]);
return $acceptedType->accepts($leftType) && $acceptedType->accepts($rightType);With no lucḱ (got bunch of internal errors).
There was a problem hiding this comment.
Sorry, I meant isSuperTypeOf (isSubTypeOf is mostly internal helper used for inversion of control). accepts() will also work if you want to allow mixed.
I tried (…) got bunch of internal errors
That's odd. The code looks fine. What errors did you get?
There was a problem hiding this comment.
It was only happening during tests, didn't find you how to enable debug mode for them...
Anyway updated to isSuperTypeOf, could you review now?
|
@Majkl578 Why not refactor it to one single class and deal with each operand? |
|
@carusogabriel |
|
isSuperTypeOf should be used to be consistent with other similar checks in
the core.
On Fri, 4 May 2018 at 21:47, Jan Tvrdík ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/Rules/Operators/OperatorRuleHelper.php
<#15 (comment)>
:
> @@ -0,0 +1,25 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Operators;
+
+use PHPStan\Type\FloatType;
+use PHPStan\Type\IntegerType;
+use PHPStan\Type\Type;
+
+class OperatorRuleHelper
+{
+
+ public static function isValidForArithmeticOperation(Type $leftType, Type $rightType): bool
+ {
+ if (!$leftType instanceof IntegerType && !$leftType instanceof FloatType) {
Sorry, I meant isSuperTypeOf (isSubTypeOf is mostly internal helper used
for *inversion of control*). accepts() will also work if you want to
allow mixed.
I tried (…) got bunch of internal errors
That's odd. The code looks fine. What errors did you get?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#15 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAGZuBqNftp-kJeuiVO00z3Ljwc4JVeFks5tvLA1gaJpZM4TzHDl>
.
--
Ondřej Mirtes
|
Majkl578
force-pushed
the
operator-arithmetic
branch
from
4c470a7 to
35a42a8
Compare
|
Btw. this doesn't support GMP, but PHPStan currently does not support resource scopes/types so it's currently impossible to implement correctly. What to do, ignore for now?; |
Majkl578
force-pushed
the
operator-arithmetic
branch
from
35a42a8 to
2baaf8e
Compare
|
Rebased. Anything needed here? I'd like to continue with other operators (comparison, bitwise). |
|
Thanks! Looking forward to the next set of rules! 👍 (I also suggest checking for strings on both sides of concat |
+.-/*///**/%.